[ENG-1177] Authenticate Obsidian user for publishing #656
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@trangdoan982: I'll conduct a full review of all the changes in this PR. Let me analyze the code for the Obsidian user authentication feature. ✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughAdds Supabase integration to the Obsidian plugin: new dependencies, build-time env injection, settings for sync and account identifiers, UI toggle to enable sync, and utilities to obtain and cache a Supabase authentication context and client. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as GeneralSettings UI
participant Plugin as Obsidian Plugin
participant Context as Supabase Context Utils
participant DB as `@repo/database`
participant Supabase as Supabase Service
User->>UI: Toggle "(BETA) Sync mode"
UI->>Plugin: save settings (syncModeEnabled=true)
Plugin->>Context: initializeSupabaseSync(plugin)
alt context obtained
Context->>Context: derive vault name, local IDs
Context->>DB: create/fetch Space record
DB-->>Context: spaceId, space data
Context->>DB: create/fetch Platform Account
DB-->>Context: accountId, user data
Context->>Supabase: create/log in client (uses SUPABASE_* env)
Supabase-->>Context: authenticated client
Context-->>Plugin: context + client cached
Plugin->>UI: show success notification
else failure
Context-->>Plugin: error
Plugin->>UI: show error notification
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/obsidian/src/index.ts (1)
37-39: Add error handling for sync initialization during startup.The
initializeSupabaseSynccall can throw an error (when context creation fails), but there's no error handling here. Unhandled promise rejections during plugin load could cause silent failures that users won't be aware of.🔎 Proposed error handling
if (this.settings.syncModeEnabled === true) { - void initializeSupabaseSync(this); + void initializeSupabaseSync(this).catch((error) => { + console.error("Failed to initialize Supabase sync on startup:", error); + // Optionally notify user + }); }apps/obsidian/scripts/compile.ts (2)
107-111: Consider removing or limiting environment variable logging.Line 111 logs the entire
dbEnvobject to the console during builds. While the Supabase anon key is meant to be public, other environment values might contain sensitive information. Consider removing this log statement or limiting it to development builds only.🔎 Suggested improvement
const dbEnv = envContents(); - console.log("dbEnv", dbEnv); + // Only log in verbose mode or remove entirely + if (process.env.VERBOSE) { + console.log("Loaded environment variables for bundling"); + }
123-131: Inconsistent fallback handling for environment variables.Lines 124-126 and 127-129 use
nullas the fallback when environment variables are missing, but line 130 uses an empty string ("") forNEXT_API_ROOT. This inconsistency could lead to different runtime behavior.Consider using a consistent fallback strategy:
🔎 Proposed consistency fix
"process.env.SUPABASE_URL": dbEnv.SUPABASE_URL ? `"${dbEnv.SUPABASE_URL}"` : "null", "process.env.SUPABASE_ANON_KEY": dbEnv.SUPABASE_ANON_KEY ? `"${dbEnv.SUPABASE_ANON_KEY}"` : "null", - "process.env.NEXT_API_ROOT": `"${dbEnv.NEXT_API_ROOT || ""}"`, + "process.env.NEXT_API_ROOT": dbEnv.NEXT_API_ROOT + ? `"${dbEnv.NEXT_API_ROOT}"` + : "null",apps/obsidian/src/utils/supabaseContext.ts (2)
21-27: Consider using cryptographically secure random generation for account IDs.The function uses
Math.random()to generate the account local ID suffix. While this may be sufficient for uniqueness, using a cryptographically secure method likecrypto.randomUUID()(which is already used for passwords on line 36) would be more robust and consistent.🔎 Proposed improvement using crypto API
const generateAccountLocalId = (vaultName: string): string => { - const randomSuffix = Math.random() - .toString(36) - .substring(2, 8) - .toUpperCase(); + // Generate 6 random hex characters using crypto API + const randomSuffix = crypto.randomUUID().substring(0, 6).toUpperCase(); return `${vaultName}-${randomSuffix}`; };
55-119: Module-level cache prevents context refresh.Line 19 defines
contextCacheat the module level, and line 58 only initializes context if the cache is null. This means:
- Context is cached for the entire lifetime of the plugin
- No mechanism exists to refresh the context if credentials change
- Multiple plugin instances (unlikely in Obsidian) would share the same cache
While this is likely fine for the Obsidian single-instance model, consider whether users might need to re-initialize or switch contexts during a session.
If context refresh is ever needed, consider:
- Adding a
clearCache()function for testing or resets- Tying the cache to the plugin instance rather than module scope
- Adding a TTL or refresh mechanism
Example cache clearing function
export const clearSupabaseContext = (): void => { contextCache = null; _loggedInClient = null; };
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
apps/obsidian/package.jsonapps/obsidian/scripts/compile.tsapps/obsidian/src/components/GeneralSettings.tsxapps/obsidian/src/constants.tsapps/obsidian/src/index.tsapps/obsidian/src/types.tsapps/obsidian/src/utils/supabaseContext.tsapps/obsidian/src/utils/syncDgNodesToSupabase.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefertypeoverinterfacein TypeScript
Use explicit return types for functions
Avoidanytypes when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability
Files:
apps/obsidian/scripts/compile.tsapps/obsidian/src/types.tsapps/obsidian/src/components/GeneralSettings.tsxapps/obsidian/src/index.tsapps/obsidian/src/utils/supabaseContext.tsapps/obsidian/src/utils/syncDgNodesToSupabase.tsapps/obsidian/src/constants.ts
apps/obsidian/**
📄 CodeRabbit inference engine (.cursor/rules/obsidian.mdc)
apps/obsidian/**: Prefer existing dependencies from apps/obsidian/package.json when adding dependencies to the Obsidian plugin
Follow the Obsidian style guide from help.obsidian.md/style-guide and docs.obsidian.md/Developer+policies for UI and code styling
Use Lucide and custom Obsidian icons alongside detailed elements to provide visual representation of features in platform-native UI
Files:
apps/obsidian/scripts/compile.tsapps/obsidian/src/types.tsapps/obsidian/src/components/GeneralSettings.tsxapps/obsidian/src/index.tsapps/obsidian/src/utils/supabaseContext.tsapps/obsidian/src/utils/syncDgNodesToSupabase.tsapps/obsidian/src/constants.tsapps/obsidian/package.json
🧠 Learnings (13)
📚 Learning: 2025-07-22T01:50:20.253Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-07-22T01:50:20.253Z
Learning: In packages/database/scripts/create_env.ts, the Vercel CLI integration uses both vercel/sdk for deployment queries and vercel CLI for environment variable pulling, with support for different variants (local, branch, production, all) and proper team/project configuration with discourse-graphs defaults.
Applied to files:
apps/obsidian/scripts/compile.ts
📚 Learning: 2025-08-31T17:38:52.779Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-08-31T17:38:52.779Z
Learning: The dbDotEnv functionality in packages/database/src/dbDotEnv.js is designed for Node.js environments only and will not be used in browser contexts.
Applied to files:
apps/obsidian/scripts/compile.ts
📚 Learning: 2025-08-31T17:37:17.173Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 400
File: packages/database/src/dbDotEnv.js:24-31
Timestamp: 2025-08-31T17:37:17.173Z
Learning: The "all" variant for SUPABASE_USE_DB should only be used in createEnv.mts and not in the dbDotEnv module. It was incorrectly added to dbDotEnv.d.ts but should be removed to maintain proper separation of concerns.
Applied to files:
apps/obsidian/scripts/compile.tsapps/obsidian/src/utils/supabaseContext.ts
📚 Learning: 2025-09-11T23:54:41.861Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 431
File: packages/database/package.json:18-22
Timestamp: 2025-09-11T23:54:41.861Z
Learning: In the discourse-graph repository's packages/database/package.json, the typesVersions mapping for "./dbDotEnv" must retain the leading "./" prefix. Removing it (as suggested by standard TypeScript documentation) fails in practice in their specific build environment/tooling setup.
Applied to files:
apps/obsidian/scripts/compile.tsapps/obsidian/package.json
📚 Learning: 2025-06-25T18:03:52.669Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Applied to files:
apps/obsidian/scripts/compile.tsapps/obsidian/package.json
📚 Learning: 2025-08-26T01:52:35.443Z
Learnt from: mdroidian
Repo: DiscourseGraphs/discourse-graph PR: 369
File: apps/roam/src/utils/pluginTimer.ts:14-21
Timestamp: 2025-08-26T01:52:35.443Z
Learning: In the discourse-graph codebase, initPluginTimer() is only called once during the runExtension flow in apps/roam/src/index.ts, so re-initialization scenarios are not a concern for this specific usage pattern.
Applied to files:
apps/obsidian/src/index.tsapps/obsidian/src/utils/syncDgNodesToSupabase.ts
📚 Learning: 2025-07-21T14:22:20.752Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 291
File: packages/database/supabase/functions/create-space/index.ts:0-0
Timestamp: 2025-07-21T14:22:20.752Z
Learning: In the discourse-graph codebase, types.gen.ts is not accessible from Supabase edge functions, requiring duplication of types and utilities when needed in the edge function environment at packages/database/supabase/functions/.
Applied to files:
apps/obsidian/src/utils/supabaseContext.ts
📚 Learning: 2025-07-08T14:48:38.048Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 272
File: packages/ui/src/lib/supabase/contextFunctions.ts:50-111
Timestamp: 2025-07-08T14:48:38.048Z
Learning: The Supabase client does not offer transaction support, so idempotent upserts with proper conflict resolution (using onConflict with ignoreDuplicates: false) are the preferred approach for multi-step database operations in packages/ui/src/lib/supabase/contextFunctions.ts. This pattern prevents orphaned records when retrying operations.
Applied to files:
apps/obsidian/src/utils/supabaseContext.ts
📚 Learning: 2025-10-18T18:58:16.100Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 504
File: apps/roam/src/utils/syncDgNodesToSupabase.ts:523-531
Timestamp: 2025-10-18T18:58:16.100Z
Learning: In `apps/roam/src/utils/syncDgNodesToSupabase.ts`, partial successes from `upsertNodesToSupabaseAsContent` and `addMissingEmbeddings` (indicated by numeric return values showing the count of successful operations) should NOT trigger backoff. Only complete failures (false) should trigger the exponential backoff mechanism. This design allows the sync process to continue making progress even when some items fail.
Applied to files:
apps/obsidian/src/utils/syncDgNodesToSupabase.ts
📚 Learning: 2025-07-08T14:51:55.299Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-07-08T14:51:55.299Z
Learning: DiscourseGraphs maintainers prefer not to add narrowly typed definitions for individual `process.env` keys to avoid unnecessary coupling.
Applied to files:
apps/obsidian/src/constants.ts
📚 Learning: 2025-11-25T00:52:36.174Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/obsidian.mdc:0-0
Timestamp: 2025-11-25T00:52:36.174Z
Learning: Applies to apps/obsidian/** : Prefer existing dependencies from apps/obsidian/package.json when adding dependencies to the Obsidian plugin
Applied to files:
apps/obsidian/package.json
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{js,ts,tsx,jsx,json} : Prefer existing dependencies from package.json when working on the Roam Research extension
Applied to files:
apps/obsidian/package.json
📚 Learning: 2025-08-27T13:59:31.143Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 377
File: packages/database/cucumber.json:3-4
Timestamp: 2025-08-27T13:59:31.143Z
Learning: In the discourse-graph monorepo, repo/* imports work correctly through standard npm package resolution where packages/*/package.json files have "name": "repo/..." and consuming apps reference these packages by name in their dependencies. The imports resolve properly in both local development and deployment environments.
Applied to files:
apps/obsidian/package.json
🧬 Code graph analysis (4)
apps/obsidian/src/components/GeneralSettings.tsx (1)
apps/obsidian/src/utils/syncDgNodesToSupabase.ts (1)
initializeSupabaseSync(4-15)
apps/obsidian/src/index.ts (1)
apps/obsidian/src/utils/syncDgNodesToSupabase.ts (1)
initializeSupabaseSync(4-15)
apps/obsidian/src/utils/supabaseContext.ts (3)
packages/database/src/dbTypes.ts (1)
Enums(1763-1778)packages/database/src/lib/contextFunctions.ts (3)
fetchOrCreateSpaceDirect(82-124)fetchOrCreatePlatformAccount(143-170)createLoggedInClient(126-141)packages/database/src/lib/client.ts (1)
DGSupabaseClient(9-9)
apps/obsidian/src/utils/syncDgNodesToSupabase.ts (2)
apps/obsidian/src/index.ts (1)
DiscourseGraphPlugin(27-351)apps/obsidian/src/utils/supabaseContext.ts (1)
getSupabaseContext(55-119)
🔇 Additional comments (9)
apps/obsidian/package.json (1)
40-42: LGTM! Dependencies added appropriately.The three new dependencies (@repo/database, @repo/utils, @supabase/supabase-js) are correctly added to support the Supabase integration feature. The workspace protocol for internal packages and catalog for external dependencies aligns with the monorepo structure.
apps/obsidian/src/types.ts (1)
37-39: LGTM! Type definitions are appropriate.The three optional fields added to the Settings type are correctly typed and follow the project's naming conventions. Making them optional ensures backward compatibility with existing settings.
apps/obsidian/src/constants.ts (1)
73-75: LGTM! Default settings are appropriate.The new default values are sensible:
syncModeEnabled: falseprovides a safe opt-in defaultspacePasswordandaccountLocalIdasundefinedallow for lazy generation when first neededapps/obsidian/scripts/compile.ts (1)
13-26: LGTM! Environment loading with clear error handling.The code properly loads database environment variables with helpful error messages guiding developers to build the database module first. The try-catch pattern appropriately distinguishes between missing modules and other errors.
apps/obsidian/src/components/GeneralSettings.tsx (3)
161-163: LGTM! State management implemented correctly.The state initialization properly handles the optional
syncModeEnabledfield with a nullish coalescing operator, and the toggle handler correctly usesuseCallbackto prevent unnecessary re-renders.Also applies to: 197-200
221-231: LGTM! Excellent error handling for sync initialization.The post-save initialization logic properly:
- Only runs when sync mode is enabled
- Awaits the async operation
- Catches and logs errors
- Provides clear user feedback for both success and failure cases
This is the correct pattern for user-initiated operations.
336-351: LGTM! UI implementation follows established patterns.The sync mode UI follows the same structure and styling as other settings in the component. The "(BETA)" prefix appropriately signals that this is an experimental feature.
apps/obsidian/src/utils/supabaseContext.ts (2)
29-40: LGTM! Secure password generation and persistence.The function correctly uses
crypto.randomUUID()for generating secure passwords and properly persists them to plugin settings. The async/await pattern is handled correctly.
123-142: The session refresh approach in the code is correct and requires no changes.The current implementation already handles session refresh properly. The
auth.getSession()method automatically attempts to refresh the access token with the refresh token if the access token is expired (per Supabase documentation). The code correctly checks for errors after callinggetSession()and clears the client if the refresh fails (e.g., due to an invalid refresh token or policy enforcement). This is the appropriate pattern.The proposed fix adds unnecessary complexity without improving the behavior—the current code already achieves the intended session renewal through Supabase's built-in auto-refresh mechanism.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/obsidian/src/utils/syncDgNodesToSupabase.ts (1)
11-14: Consider removing sensitive identifiers from production logs.The success log currently includes
spaceIdanduserId, which may expose sensitive information in production environments. Consider logging a generic success message or making the detailed log conditional on a development flag.
🧹 Nitpick comments (3)
apps/obsidian/src/components/GeneralSettings.tsx (1)
221-231: Consider initializing sync only when enabled state changes.Currently,
initializeSupabaseSyncis called every time settings are saved ifsyncModeEnabledis true. While the context cache prevents redundant initialization, it's more efficient to only call this when sync mode is toggled from false to true.🔎 Suggested optimization
Track the previous sync state and only initialize on enable:
const [syncModeEnabled, setSyncModeEnabled] = useState<boolean>( plugin.settings.syncModeEnabled ?? false, ); + const [previousSyncMode, setPreviousSyncMode] = useState<boolean>( + plugin.settings.syncModeEnabled ?? false, + ); // ... in handleSave ... plugin.settings.syncModeEnabled = syncModeEnabled; // ... save other settings ... await plugin.saveSettings(); new Notice("General settings saved"); setHasUnsavedChanges(false); - if (syncModeEnabled) { + if (syncModeEnabled && !previousSyncMode) { try { await initializeSupabaseSync(plugin); new Notice("Sync mode initialized successfully"); + setPreviousSyncMode(true); } catch (error) { console.error("Failed to initialize sync mode:", error); new Notice( `Failed to initialize sync mode: ${error instanceof Error ? error.message : String(error)}`, ); } } + if (!syncModeEnabled && previousSyncMode) { + setPreviousSyncMode(false); + }apps/obsidian/src/utils/supabaseContext.ts (2)
21-27: Prefer crypto-based randomness for unique identifiers.The function uses
Math.random()to generate the account local ID suffix, which is not cryptographically secure and has potential collision risks. Consider usingcrypto.randomUUID()(already used forspacePasswordat line 36) for better uniqueness guarantees.🔎 Proposed improvement
const generateAccountLocalId = (vaultName: string): string => { - const randomSuffix = Math.random() - .toString(36) - .substring(2, 8) - .toUpperCase(); + const randomSuffix = crypto.randomUUID().substring(0, 6).toUpperCase(); return `${vaultName}-${randomSuffix}`; };
135-140: Consider recreating client immediately on session error.When session refresh fails, the function sets
loggedInClientto null and returns null, requiring the caller to retry. For better resilience, consider recreating the client immediately within the same call.🔎 Suggested improvement
} else { // renew session const { error } = await loggedInClient.auth.getSession(); if (error) { - loggedInClient = null; + // Recreate client on session error + const context = await getSupabaseContext(plugin); + if (context === null) { + loggedInClient = null; + return null; + } + loggedInClient = await createLoggedInClient( + context.platform, + context.spaceId, + context.spacePassword, + ); } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
apps/obsidian/package.jsonapps/obsidian/scripts/compile.tsapps/obsidian/src/components/GeneralSettings.tsxapps/obsidian/src/constants.tsapps/obsidian/src/index.tsapps/obsidian/src/types.tsapps/obsidian/src/utils/supabaseContext.tsapps/obsidian/src/utils/syncDgNodesToSupabase.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/obsidian/**
📄 CodeRabbit inference engine (.cursor/rules/obsidian.mdc)
apps/obsidian/**: Prefer existing dependencies from apps/obsidian/package.json when adding dependencies to the Obsidian plugin
Follow the Obsidian style guide from help.obsidian.md/style-guide and docs.obsidian.md/Developer+policies for UI and code styling
Use Lucide and custom Obsidian icons alongside detailed elements to provide visual representation of features in platform-native UI
Files:
apps/obsidian/package.jsonapps/obsidian/src/components/GeneralSettings.tsxapps/obsidian/scripts/compile.tsapps/obsidian/src/constants.tsapps/obsidian/src/index.tsapps/obsidian/src/types.tsapps/obsidian/src/utils/supabaseContext.tsapps/obsidian/src/utils/syncDgNodesToSupabase.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefertypeoverinterfacein TypeScript
Use explicit return types for functions
Avoidanytypes when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability
Files:
apps/obsidian/src/components/GeneralSettings.tsxapps/obsidian/scripts/compile.tsapps/obsidian/src/constants.tsapps/obsidian/src/index.tsapps/obsidian/src/types.tsapps/obsidian/src/utils/supabaseContext.tsapps/obsidian/src/utils/syncDgNodesToSupabase.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/obsidian.mdc:0-0
Timestamp: 2025-11-25T00:52:36.174Z
Learning: Applies to apps/obsidian/** : Prefer existing dependencies from apps/obsidian/package.json when adding dependencies to the Obsidian plugin
📚 Learning: 2025-11-25T00:52:36.174Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/obsidian.mdc:0-0
Timestamp: 2025-11-25T00:52:36.174Z
Learning: Applies to apps/obsidian/** : Prefer existing dependencies from apps/obsidian/package.json when adding dependencies to the Obsidian plugin
Applied to files:
apps/obsidian/package.json
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{js,ts,tsx,jsx,json} : Prefer existing dependencies from package.json when working on the Roam Research extension
Applied to files:
apps/obsidian/package.json
📚 Learning: 2025-06-25T18:03:52.669Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Applied to files:
apps/obsidian/package.jsonapps/obsidian/scripts/compile.ts
📚 Learning: 2025-08-27T13:59:31.143Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 377
File: packages/database/cucumber.json:3-4
Timestamp: 2025-08-27T13:59:31.143Z
Learning: In the discourse-graph monorepo, repo/* imports work correctly through standard npm package resolution where packages/*/package.json files have "name": "repo/..." and consuming apps reference these packages by name in their dependencies. The imports resolve properly in both local development and deployment environments.
Applied to files:
apps/obsidian/package.json
📚 Learning: 2025-09-11T23:54:41.861Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 431
File: packages/database/package.json:18-22
Timestamp: 2025-09-11T23:54:41.861Z
Learning: In the discourse-graph repository's packages/database/package.json, the typesVersions mapping for "./dbDotEnv" must retain the leading "./" prefix. Removing it (as suggested by standard TypeScript documentation) fails in practice in their specific build environment/tooling setup.
Applied to files:
apps/obsidian/package.jsonapps/obsidian/scripts/compile.ts
📚 Learning: 2025-08-31T17:38:52.779Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-08-31T17:38:52.779Z
Learning: The dbDotEnv functionality in packages/database/src/dbDotEnv.js is designed for Node.js environments only and will not be used in browser contexts.
Applied to files:
apps/obsidian/scripts/compile.ts
📚 Learning: 2025-07-22T01:50:20.253Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-07-22T01:50:20.253Z
Learning: In packages/database/scripts/create_env.ts, the Vercel CLI integration uses both vercel/sdk for deployment queries and vercel CLI for environment variable pulling, with support for different variants (local, branch, production, all) and proper team/project configuration with discourse-graphs defaults.
Applied to files:
apps/obsidian/scripts/compile.ts
📚 Learning: 2025-08-31T17:37:17.173Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 400
File: packages/database/src/dbDotEnv.js:24-31
Timestamp: 2025-08-31T17:37:17.173Z
Learning: The "all" variant for SUPABASE_USE_DB should only be used in createEnv.mts and not in the dbDotEnv module. It was incorrectly added to dbDotEnv.d.ts but should be removed to maintain proper separation of concerns.
Applied to files:
apps/obsidian/scripts/compile.tsapps/obsidian/src/utils/supabaseContext.ts
📚 Learning: 2025-09-01T19:09:07.052Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-09-01T19:09:07.052Z
Learning: User prefers to avoid making types explicit in each require statement due to anti-DRY concerns when working with module imports in TypeScript.
Applied to files:
apps/obsidian/scripts/compile.ts
📚 Learning: 2025-07-08T14:51:55.299Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-07-08T14:51:55.299Z
Learning: DiscourseGraphs maintainers prefer not to add narrowly typed definitions for individual `process.env` keys to avoid unnecessary coupling.
Applied to files:
apps/obsidian/src/constants.ts
📚 Learning: 2025-08-26T01:52:35.443Z
Learnt from: mdroidian
Repo: DiscourseGraphs/discourse-graph PR: 369
File: apps/roam/src/utils/pluginTimer.ts:14-21
Timestamp: 2025-08-26T01:52:35.443Z
Learning: In the discourse-graph codebase, initPluginTimer() is only called once during the runExtension flow in apps/roam/src/index.ts, so re-initialization scenarios are not a concern for this specific usage pattern.
Applied to files:
apps/obsidian/src/index.tsapps/obsidian/src/utils/syncDgNodesToSupabase.ts
📚 Learning: 2025-07-21T14:22:20.752Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 291
File: packages/database/supabase/functions/create-space/index.ts:0-0
Timestamp: 2025-07-21T14:22:20.752Z
Learning: In the discourse-graph codebase, types.gen.ts is not accessible from Supabase edge functions, requiring duplication of types and utilities when needed in the edge function environment at packages/database/supabase/functions/.
Applied to files:
apps/obsidian/src/utils/supabaseContext.ts
📚 Learning: 2025-07-08T14:48:38.048Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 272
File: packages/ui/src/lib/supabase/contextFunctions.ts:50-111
Timestamp: 2025-07-08T14:48:38.048Z
Learning: The Supabase client does not offer transaction support, so idempotent upserts with proper conflict resolution (using onConflict with ignoreDuplicates: false) are the preferred approach for multi-step database operations in packages/ui/src/lib/supabase/contextFunctions.ts. This pattern prevents orphaned records when retrying operations.
Applied to files:
apps/obsidian/src/utils/supabaseContext.ts
📚 Learning: 2025-05-30T14:49:24.016Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Applied to files:
apps/obsidian/src/utils/supabaseContext.ts
📚 Learning: 2025-11-23T23:53:43.094Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-11-23T23:53:43.094Z
Learning: In `apps/roam/src/utils/supabaseContext.ts`, the Roam URL normalization uses a targeted string replacement `url.replace("/?server-port=3333#/", "/#/")` rather than URL parsing to avoid impacting unforeseen URL patterns. This conservative approach is intentional to handle only the specific known case.
Applied to files:
apps/obsidian/src/utils/supabaseContext.ts
🧬 Code graph analysis (3)
apps/obsidian/src/components/GeneralSettings.tsx (1)
apps/obsidian/src/utils/syncDgNodesToSupabase.ts (1)
initializeSupabaseSync(4-15)
apps/obsidian/src/index.ts (1)
apps/obsidian/src/utils/syncDgNodesToSupabase.ts (1)
initializeSupabaseSync(4-15)
apps/obsidian/src/utils/syncDgNodesToSupabase.ts (2)
apps/obsidian/src/index.ts (1)
DiscourseGraphPlugin(27-351)apps/obsidian/src/utils/supabaseContext.ts (1)
getSupabaseContext(55-119)
🔇 Additional comments (2)
apps/obsidian/scripts/compile.ts (1)
121-129: LGTM: Standard Supabase client configuration.Injecting
SUPABASE_ANON_KEYinto the client bundle is the correct approach for Supabase client-side authentication. The anon key is designed to be public and is properly protected by Row Level Security (RLS) policies on the database side.apps/obsidian/src/utils/supabaseContext.ts (1)
62-76: Vault name extraction logic is defensive and appropriate.The code properly checks for
getName()availability before using it, and falls back to adapter-based extraction when needed. The fallback to"obsidian-vault"ensures the function always returns a valid name.
| } | ||
|
|
||
| const spaceId = spaceResult.data.id; | ||
| const userId = await fetchOrCreatePlatformAccount({ |
There was a problem hiding this comment.
I'm following Roam pattern here to create both Anonymous User and the real user (name here is vaultName instead of "Anonymous of space [x]". But I would say we don't need this for Obsidian since each vault is a space, and is owned by only one person? hence all the handling would go through only need to go through 1 account? I can just comment out this part of the code to delete this real user creation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.